Skip to content

fix: prevent fetcher formData flicker and eliminate state.fetchers mutations#15028

Merged
brophdawg11 merged 10 commits into
devfrom
agents/fix-bug-14506
May 13, 2026
Merged

fix: prevent fetcher formData flicker and eliminate state.fetchers mutations#15028
brophdawg11 merged 10 commits into
devfrom
agents/fix-bug-14506

Conversation

@brophdawg11
Copy link
Copy Markdown
Contributor

Problem

Fixes #14506 — fetcher formData briefly becomes undefined before new loaderData is available, causing UI flicker in optimistic update patterns.

Root cause

In handleFetcherAction, after the action resolved and loaders were awaited, the code called:

updateState({ fetchers: new Map(state.fetchers) })  // hands Map M to React
// ... async loaders run ...
state.fetchers.set(key, doneFetcher)  // mutates Map M that React already holds!

Because startTransition renders are low-priority, React could render Map M between these two lines — seeing the idle formData: undefined from the done-fetcher but still with stale loaderData. This caused a one-frame flicker.

Fix (commit 1)

Defer the done-fetcher assignment to the final atomic updateState call:

let finalFetchers = new Map(state.fetchers);
finalFetchers.set(key, getDoneFetcher(actionResult.data));  // include done-fetcher atomically
updateState({ fetchers: finalFetchers, loaderData, errors });

A regression test is included that uses startTransition semantics to verify the fetcher is never exposed in an intermediate inconsistent state.

Refactor (commit 2)

The fix revealed a broader pattern: state.fetchers was being mutated directly in ~11 places across 7 functions throughout the router. Any of these mutations could cause the same class of bug if React happened to render between the mutation and the next updateState.

This commit eliminates all direct state.fetchers mutations:

  • updateState: delete idle mounted fetchers only after subscribers are notified (so subscribers capture idle fetcher data); the pre-notification deletion was incorrect
  • HandleLoadersResult: added optional fetchers field so abortStaleFetchLoads / markFetchRedirectsDone results are committed atomically through startNavigation → completeNavigation
  • getUpdatedRevalidatingFetchers: builds a copy, never mutates state.fetchers
  • handleFetcherAction: builds updatedFetchers / finalFetchers maps; redirect paths use state = { ...state, fetchers: withDoneFetcher() }
  • updateFetcherState: builds new Map before updateState
  • setFetcherError: builds new Map before updateState
  • deleteFetcher: no longer deletes from state.fetchers; callers build their own snapshot
  • markFetchersDone / markFetchRedirectsDone / abortStaleFetchLoads: accept a fetchers: Map parameter and operate on it
  • processLoaderData: accepts a workingFetchers: Map parameter

Testing

All 98 fetcher unit tests pass, including the new regression test. The pre-existing dom-export-test.tsx failure is unrelated to these changes.

@brophdawg11 brophdawg11 changed the title fix: prevent fetcher formData flicker and eliminate state.fetchers mutations (#14506) fix: prevent fetcher formData flicker and eliminate state.fetchers mutations May 8, 2026
brophdawg11 and others added 2 commits May 8, 2026 16:19
…ction (#14506)

When a fetcher action completes and revalidation loaders run, the router
was calling state.fetchers.set(key, doneFetcher) BEFORE checking for
redirects and before the final updateState/completeNavigation call. This
mutated the same Map object that had already been handed to React via a
prior updateState() call.

Because React renders startTransition updates asynchronously, if React
rendered between the mutation and the next updateState(), it would see:
- fetcher.formData === undefined (already idle)
- loaderData === old value (not yet updated)

This caused a brief flicker in optimistic UI patterns like:
  fetcher.formData?.get('status') ?? item.status

The fix defers the fetcher idle-state assignment to the final state
update (non-redirect path) so it's committed atomically alongside the
new loaderData. For redirect paths the mutation is preserved (needed so
the redirect navigation's completeNavigation sees the idle fetcher).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Build a new Map whenever fetcher state needs to change instead of
mutating the existing map in-place.  The existing map may already have
 setState) and
React renders asynchronously, so mutating it after the fact is unsafe.

Key changes:
- updateState: delete idle mountedFetchers only AFTER subscribers are
  notified (so subscribers can capture idle fetcher data), not before
- HandleLoadersResult now carries an optional fetchers map so that
  abortStaleFetchLoads / markFetchRedirectsDone results are committed
 completeNavigation
- getUpdatedRevalidatingFetchers: build a copy, don't mutate state.fetchers
- handleFetcherAction: build updatedFetchers / finalFetchers maps instead
  of mutating state.fetchers; redirect paths advance state.fetchers via
  state = { ...state, fetchers: withDoneFetcher() }
- updateFetcherState: build new Map before calling updateState
- setFetcherError: build new Map before calling updateState
- deleteFetcher: no longer deletes from state.fetchers; callers that need
  a React-facing snapshot must build their own Map and exclude the key
- markFetchersDone / markFetchRedirectsDone / abortStaleFetchLoads: accept
  a fetchers Map parameter and operate on that instead of state.fetchers
- processLoaderData: accepts workingFetchers Map parameter

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@brophdawg11 brophdawg11 force-pushed the agents/fix-bug-14506 branch from 0298a84 to cc1bffd Compare May 8, 2026 20:19
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

✅ Change File Found

A change file file exists in this PR. Thanks!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

Preview Build Available

Preview builds have been created for this PR. You can install react-router using:

pnpm install "remix-run/react-router#preview/pr-15028&path:packages/react-router"

And/or install other packages via:

pnpm install "remix-run/react-router#preview/pr-15028&path:packages/react-router-dev"
pnpm install "remix-run/react-router#preview/pr-15028&path:packages/react-router-express"
pnpm install "remix-run/react-router#preview/pr-15028&path:packages/react-router-node"
pnpm install "remix-run/react-router#preview/pr-15028&path:packages/react-router-serve"

These preview builds will be updated automatically as you push new commits.

brophdawg11 and others added 6 commits May 11, 2026 12:46
Since state.fetchers is now always replaced with a new Map (never
mutated in place), we can key useMemo on that reference. The expensive
Array.from + spread only runs when fetchers actually change, not on
every unrelated navigation or state update.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The original test checked subscriber states and could never  thefail
subscriber sees state after each updateState call, not the intermediate
mutation. The real bug is that state.fetchers.set(key, doneFetcher)
mutates the Map reference already handed to React's concurrent renderer.

The new test captures the Map reference from the subscriber when the
fetcher enters the 'loading' state, then asserts that reference was NOT
mutated to 'idle' after the fetch completes. This fails on the dev branch
(before the fix) with 'Expected: loading, Received: idle' and passes with
the immutability refactor in place.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds four additional regression tests alongside the existing action+loader
test, covering every code path that previously mutated state.fetchers in
place after handing the Map reference to React:

- Fetcher GET load: updateFetcherState() called twice (loading then done)
  mutated the loading Map via the second state.fetchers.set() call.

- Fetcher revalidation during navigation: processLoaderData() mutated the
  loading Map (handed to subscribers during getUpdatedRevalidatingFetchers)
  when marking revalidating fetchers done after loaders completed.

- Fetcher loader redirect: markFetchersDone() inside completeNavigation
  mutated the loading Map handed to subscribers before the redirect finished.

- Fetcher action redirect: markFetchersDone() inside completeNavigation
  mutated the loading Map (post-action-redirect) handed to subscribers
  before the redirect navigation completed.

Each test captures the Map reference from the subscriber at the relevant
intermediate state, then asserts after completion that the Map still
reflects that intermediate state (not the final idle state). All five
tests fail on the dev branch and pass with the immutability refactor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
React's useId() can produce IDs like _r_11_ (digits only, no trailing
letter), but the regex was /^_r_[0-9]?[a-z]_/ which required exactly
one alpha character. Broaden to /^_r_[0-9a-z]+_/ to match any
alphanumeric suffix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md Outdated
@brophdawg11 brophdawg11 marked this pull request as ready for review May 12, 2026 21:12
Comment thread packages/react-router/.changes/patch.fix-fetcher-formdata-flicker.md Outdated
@brophdawg11 brophdawg11 merged commit 44c3478 into dev May 13, 2026
7 checks passed
@brophdawg11 brophdawg11 deleted the agents/fix-bug-14506 branch May 13, 2026 13:28
@github-actions
Copy link
Copy Markdown
Contributor

The preview branch preview/pr-15028 has been deleted now that this PR is merged/closed.

@ryanflorence ryanflorence mentioned this pull request May 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Hello there,

We just published version 7.15.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fetcher sometimes transitions to idle before new loader data is available

1 participant